Support site install on Ubuntu 24.04 + PHP 8.3 #3075
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes to get the install working: * Upgrade base OS version (default PHP follows along) * Remove MySQL version overrides since MySQL 8.0 is already the default for Ubuntu 24.04 * Update all PHP FS paths to expect 8.3 instead of 7.0 * Fix mandatory puppet syntax changes for puppet 8 (class names can't have dashes, file mode variables are strings, facter variables come from a dict) * Update some python utilities to new default version python3.12, both in the base install and in CircleCI * Stop trying to instantiate cloudwatch config (which doesn't work right now for us anyway) * Work around rsyslog no longer supporting SysVInit syntax - just start the binary on container startup * Fix a small hook bug in perform_end_of_turn_die_actions() - previous versions of PHP would let you | a bool with an array and get something truthy; that doesn't fly in PHP 8.3 Changes to get PHPUnit working: * Apply puppet version fixes to circleci puppet * Don't report useless tests, since there are a lot of noop tests and it'll be annoying * Fix namespace of PHPUnit to match how PHPUnit 9 does it, and also apply required type specification for setUp() and tearDown() functions * Comment out a couple of die tests which use readAttribute() - that's not allowed any more, so we'll have to figure out what to do instead * PHPUnit 9 doesn't reset global variables between tests any more; fix a couple of tests which use global variables so the right thing happens (TODO: there may be more of these) Changes to get UI tests working: * Install phantomjs via npm on CircleCI nodes * Work around incompatibility between phantomjs and newer OpenSSL by disabling OpenSSL in the phantomjs invocation
Contributor
Author
cgolubi1
commented
Jan 4, 2026
Comment on lines
-34
to
+35
| function_name_re = re.compile('^(%s\.\S+) = function\(' % modname) | ||
| test_name_re = re.compile('^(test|asyncTest)\("test_([^"]+)"') | ||
| function_name_re = re.compile('^(%s.[^ \t\n\r\f\v]+) = function[(]' % modname) | ||
| test_name_re = re.compile('^(test|asyncTest)[(]"test_([^"]+)"') |
Contributor
Author
There was a problem hiding this comment.
Drive-by comment: i could not tell you why python 3.12 has decided it's so much cleaner to use [^ \t\n\r\f\v] than \S (and [(] than \(). This seems like a mistake someone is going to fix at some point, but the SyntaxWarnings are pretty distracting, and we don't have to touch this code all that often, so i just decided to go with what python 3.12 told me to do.
Contributor
|
This pull request looks good to me. I'm fine with having those tests commented out for a bit, so that we can resolve the issue in a subsequent pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2936 (when finished).
Changes to get the install working:
perform_end_of_turn_die_actions()- previous versions of PHP would let you | a bool with an array and get something truthy; that doesn't fly in PHP 8.3Changes to get PHPUnit working:
Changes to get UI tests working:
Shadowshade: it'd be great if you could take a look at this:
readAttribute(), do you have any concerns about the approach?readAttribute()test problem? Do you think it's a blocker? I can try to think about other ways to test the internal state you're trying to test, but i kind of think this is something we're going to have to improve over time. I'm personally not that worried about it, because it's only two tests, and my experience has been that testing the externally-visible state of games works pretty well. But i'm sure you use the tests for various things before submitting PRs in the first place.perform_end_of_turn_die_actions(), some of the responder tests were failing withTypeError: Unsupported operand types: bool | arrayin a case in which a button hado(V)?at the end of a round. I think it's pretty clear what the issue was (as noted above in the commit message), and i don't have concerns about the fix, but LMK if you do.Obviously i'll need to put up a dev site and run some replay tests and stuff, but i wanted to start out with you taking a look at the change now that CircleCI is passing.
One note in passing: it seems like newer versions of PHP have better support for type hints (that's what i had to add to the
setUp()andtearDown()functions). I actually think making use of type hints in our PHP code could do us a lot of good in terms of avoiding surprising problems caused by functions that turn out to be more polymorphic than we thought they were. If you agree, let's cut an issue to start adding them down the line as time permits.